Skip to content

Add ILoggerFactory to StreamableHttpServerTransport#1213

Merged
stephentoub merged 5 commits intomainfrom
copilot/add-logger-factory-to-streamable-http
Jan 30, 2026
Merged

Add ILoggerFactory to StreamableHttpServerTransport#1213
stephentoub merged 5 commits intomainfrom
copilot/add-logger-factory-to-streamable-http

Conversation

Copy link
Contributor

Copilot AI commented Jan 30, 2026 •
edited
Loading

  • Add ILoggerFactory parameter to StreamableHttpServerTransport constructor
  • Pass logger to StreamableHttpPostTransport via primary constructor
  • Add logging for exceptions in HandleStoreStreamDisposalAsync
  • Update StreamableHttpHandler to pass ILoggerFactory
  • Address PR feedback:
    • Restored primary constructor pattern in StreamableHttpPostTransport
    • Moved using declaration before try block in HandleStoreStreamDisposalAsync
    • Removed unnecessary catch (OperationCanceledException) block
Original prompt

This section details on the original issue you should resolve

<issue_title>Add ILoggerFactory to StreamableHttpServerTransport</issue_title>
<issue_description>We cannot always bubble up unhandled exceptions through the public API now that code can continue running after

I think we're going to have to bite the bullet and add a new constructor to StreamableHttpServerTransport that takes an optional ILoggerFactory, so we can at least log errors if _sseEventStreamWriter!.DisposeAsync() throws in HandleStreamWriterDisposalAsync.

We don't have to do it in this PR, but I think we should follow up quickly if not.

Originally posted by @halter73 in #1077 (comment)
</issue_description>

Comments on the Issue (you are @copilot in this section)

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits January 30, 2026 04:50
@stephentoub
Add ILoggerFactory parameter to StreamableHttpServerTransport constru…
8d1d467 
…ctor

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
@stephentoub
Address code review feedback: fix logger field capture and remove unu…
f141d04 
…sed field

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Copilot AI changed the title [WIP] Add ILoggerFactory to StreamableHttpServerTransport constructor Add ILoggerFactory to StreamableHttpServerTransport Jan 30, 2026
Copilot AI requested a review from stephentoub January 30, 2026 05:03
stephentoub
stephentoub reviewed Jan 30, 2026
View reviewed changes
@stephentoub stephentoub marked this pull request as ready for review January 30, 2026 16:19
@stephentoub stephentoub requested a review from halter73 January 30, 2026 16:19
@stephentoub
Restore primary constructor and fix HandleStoreStreamDisposalAsync
cff6d2e 
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Copilot AI requested a review from stephentoub January 30, 2026 16:24
@stephentoub stephentoub merged commit 5084a79 into main Jan 30, 2026
10 checks passed
@stephentoub stephentoub deleted the copilot/add-logger-factory-to-streamable-http branch January 30, 2026 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@halter73 halter73 halter73 approved these changes

@stephentoub stephentoub Awaiting requested review from stephentoub

Assignees

@stephentoub stephentoub

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add ILoggerFactory to StreamableHttpServerTransport

3 participants

@halter73 @stephentoub